Skip to content

[feat/#466] TownSelectBottomSheet 구현#470

Open
seojoozero wants to merge 12 commits intodevelopfrom
feat/#466-town-select-bottom-sheet
Open

[feat/#466] TownSelectBottomSheet 구현#470
seojoozero wants to merge 12 commits intodevelopfrom
feat/#466-town-select-bottom-sheet

Conversation

@seojoozero
Copy link
Copy Markdown
Contributor

📄 작업 내용

  • TownSelectBottomSheet를 구현했습니다.
  • 부모 뷰(AIRecommendPromptView)에 연결했습니다.
  • 스켈레톤 뷰 연결했습니다.
구현 내용 iPhone 13 mini iPhone 16 pro
동네 선택 바텀 시트 구현

💻 주요 코드 설명

스켈레톤 뷰 연결 후 이슈 해결

...
if !store.state.isTownLoading {
    divider
}
...

톡방에 공유했던 이슈(바텀시트에서 리스트를 불러올 때 리스트 위쪽 divider가 겹쳐서 화면에 두껍게 나타나는 ..) 위처럼 해결했어유 !!!

🔗 연결된 이슈

👀 기타 더 이야기해볼 점

  • '완료' 버튼은 API 연결하면서 같이 구현하겠습니다.
  • JGDTopTownRowJGDSubTownRow 두 가지를 JGD 폴더 내의 Component에서 Global의 Component로 이동했습니다.
  • 지금 동네 선택 뷰에 불러와지는 동네 리스트 개수와 스켈레톤 뷰에 표시되는 개수가 달라서 살짝 어색한 것 같은데, 다들 어떻게 생각하시는지 ? ? 해결 방법이 있나요 ?

@seojoozero seojoozero requested a review from a team March 21, 2026 10:55
@seojoozero seojoozero self-assigned this Mar 21, 2026
@seojoozero seojoozero added 🐰 jooyoung 주영이가함! 🛠️ feat 새로운 기능 구현 시 사용 labels Mar 21, 2026
@seojoozero seojoozero linked an issue Mar 21, 2026 that may be closed by this pull request
2 tasks
Copy link
Copy Markdown
Collaborator

@SeungWon1125 SeungWon1125 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!

그 바텀시트에 띄울 뷰가 서브뷰임에도 불구하고, Store를 직접 참조하여 사용하고 있는데, 그 부분만 수정하면 좋을 거 같습니다!

Comment on lines +39 to +44
SolplyMainButton(
title: "완료",
isEnabled: true
) {

}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

별 건 아닌데, 어떤 작업 연결해야하는지 TODO 주석으로 남겨주세용

Comment on lines +13 to +16

@Environment(\.dismiss) private var dismiss
@StateObject private var store = AIRecommendPromptStore()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 뷰는 AIRecommendPromptView의 서브뷰 역할을 하기 때문에, Store를 직접 들고 있는 건 적절하지 않아 보입니다!

@StateObject로 선언하면 이 뷰가 Store의 생명주기를 직접 수요하게 되는데,, 서브뷰가 Store를 소유하는 건 의도한 구조가 아닐 거라고 생각이 드네욥, @ObservedObject로 바꾼다고 하더라도, 서브뷰가 Store자체를 참조하게 되면 MVI 패턴에서 의도하는 단방향 데이터 흐름이 깨진다고 생각합니다!

Store를 여러 뷰에서 직접 참조하게 되면 어느 뷰에서 상태 변경이 발생했는지 추적하기 어려워지고, 뷰 간 결합도도 높아지기 때문에 유지보수가 어려워질 수 있을 거 같아요

MVI 패턴에 더 맞는 방향은, Store를 넘겨주는 것보단, 실행할 클로저를 프로퍼티로 선언하고, 부모뷰(AIRecommendPromptView)에서 초기화할 때 해당 클로저를 전달해주는 방식을 사용하는 게 더 적절한 거 같습니다. 요렇게 하면 서브뷰는 의도만 전달하는 역할에 집중하게 되고, 실제 상태를 변경하는 거는 부모 뷰에서만 일어나도록 책임을 분리할 수 있습니다!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예를 들어 지금 이 컴포에서 저 동네 리스트를 띄우기 위해 필요한 배열이라던가, '완료' 버튼을 누르면 실행할 action이라던가 등등.. 을 프로퍼티로 정의한 뒤에 부모뷰에서 초기화할 때 넘겨주는 방식으로 하면 결합도를 낮출 수 있을 거 같아요

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MVI 쓰면서 멀어진 것들 : @State, @StateObject.......

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StateObject는 ObservableObject 객체를 최초 생성하고 생명주기를 소유하게 되는 거라서, View에서 @StateObject로 Store를 초기화 해서 최초 1회 생성을 보장하는 겁니다. 이미 우리 Store 초기화할 때 @StateObject로 해왔는뎁.. @dudwntjs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image 웁씨 세상에

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서브 뷰가 Store를 직접 가졌을 때, 우리가 의도한 단방향 데이터 흐름이 깨진다는 말이 이해가 갔습니다 !
수정해보았는데, 이런 식으로 하는 게 맞을까 싶어요. 코드 다시 한 번 봐주시면 감사하겠습니다 ~~

Comment on lines +68 to +71
Image(.xIconSm)
.resizable()
.frame(width: 24.adjusted, height: 24.adjusted)
.foregroundStyle(.gray800)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image(아이콘) 리사이저블 밑에 .aspectRatio(contentMode: .fit) 달아주세요!

Comment on lines +86 to +88
private var townListView: some View {
VStack(spacing: 0) {
ForEach(store.state.townList, id: \.self) { town in
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VStack alignment 명시!

Copy link
Copy Markdown
Contributor

@dudwntjs dudwntjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

완전 재빠르시군요 아빠가 코멘트 다 달아서 전 어푸 드립니듀
Image

Comment on lines +13 to +16

@Environment(\.dismiss) private var dismiss
@StateObject private var store = AIRecommendPromptStore()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MVI 쓰면서 멀어진 것들 : @State, @StateObject.......

Copy link
Copy Markdown
Collaborator

@SeungWon1125 SeungWon1125 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store 분리 너무 좋슴니다~!

코멘트 단 부분만 한번 확인해주세용

Comment on lines +16 to +42
@State private var selectedTown: Town?
@State private var selectedSubTown: SubTown?

private let isTownLoading: Bool
private let townList: [Town]

private let initialTown: Town?
private let initialSubTown: SubTown?

private let onAppear: (() -> Void)?
private let onComplete: ((Town?, SubTown?) -> Void)?

init(
isTownLoading: Bool,
townList: [Town],
initialTown: Town? = nil,
initialSubTown: SubTown? = nil,
onAppear: (() -> Void)? = nil,
onComplete: ((Town?, SubTown?) -> Void)? = nil
) {
self.isTownLoading = isTownLoading
self.townList = townList
self.initialTown = initialTown
self.initialSubTown = initialSubTown
self.onAppear = onAppear
self.onComplete = onComplete
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 selectedTown이랑 selectedSubTown을 옵셔널로 두고, initialTown, initialSubTown을 두신 건 초기값 때문인 건가요? selectedTown이랑 selectedSubTown은 선택이 안 되어있는 상황은 없으니까 옵셔널로 두지 않아도 될 거 같습니다! 그리고

init(
    isTownLoading: Bool,
    townList: [Town],
    initialTown: Town,
    initialSubTown: SubTown,
    onAppear: (() -> Void? = nil,
    onComplete: ((Town, SubTown) -> Void)? = nil
) {
    self.isTownLoading = isTownLoading
    self.townList = townList
    self.onAppear = onAppear
    self.onComplete = onComplete
    self._selectedTown = State(initialValue: initialTown)
    self._selectedSubTown = State(initialValue: initialSubTown)
}

이렇게 초기값을 설정해주는 것은 어떤가요?

Comment on lines +33 to +35
onAppear: (() -> Void)? = nil,
onComplete: ((Town?, SubTown?) -> Void)? = nil
) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그리고 onComplete 클로저에 Town이랑 SubTownselectedTown, selectedSubTown 때문에 옵셔널인 거 같은데, 얘도 옵셔널이 아니어도 괜찮을 거 같아요! 혹시 제가 놓친 부분이 있다면 알려주세용

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🛠️ feat 새로운 기능 구현 시 사용 🐰 jooyoung 주영이가함!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] 동네 선택 바텀시트 구현

3 participants